Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

As per #855 issue, use initializer list #121

Closed
wants to merge 1 commit into from

Conversation

shivangvijay
Copy link
Contributor

moveit/moveit2#855, Making code cleaner for rmw_zenoh repository.

@Yadunund
Copy link
Member

Yadunund commented Mar 4, 2024

@shivangvijay thanks for the PR but i'm afraid these are not valid changes. There is no member name to initialize and the original implementation has no issues imo. If you would like to contribute, please look at the Issues page for things to implement.

@Yadunund Yadunund closed this Mar 4, 2024
@shivangvijay
Copy link
Contributor Author

shivangvijay commented Mar 4, 2024

Sorry I found my implementation mistake. I wanted to start with a 'good first issue'. However, I couldn't test it locally because I have ROS2 Humble, and I encountered issues while building rmw_zenoh. I didn't invest much effort into resolving this issue in Humble because the documentation specifies that the repository only supports ROS2 Rolling and Iron.

There are no issues with the original implementation; I simply wanted to make the code cleaner. I will create another 'good first issue'-related pull request. If you believe it will make the code cleaner, please accept it. I have started exploring rmw_zenoh repository issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants